Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Renamed variables in UML diagrams and comments #914

Merged
merged 24 commits into from
Sep 17, 2024
Merged

Renamed variables in UML diagrams and comments #914

merged 24 commits into from
Sep 17, 2024

Conversation

marvinheintze
Copy link
Contributor

Resolves #675
Renamed
ActivityStartTrigger to Activation,
ScheduleTriggerMessage to ScheduleActivation,
CompletionMessage to Completion
in UML diagrams and in comments.

@marvinheintze marvinheintze added the documentation Improvements or additions to documentation label Aug 20, 2024
@marvinheintze marvinheintze self-assigned this Aug 20, 2024
@marvinheintze marvinheintze changed the title Mh/#675 Adapting UML diagrams and docs Renamed variables in UML diagram and comments Aug 20, 2024
@marvinheintze marvinheintze marked this pull request as ready for review August 20, 2024 11:36
@marvinheintze marvinheintze changed the title Renamed variables in UML diagram and comments Renamed variables in UML diagrams and comments Aug 20, 2024
sebastian-peter
sebastian-peter reviewed Aug 21, 2024
View reviewed changes
Copy link
Member

@sebastian-peter sebastian-peter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you look into the failed powerflow messages, like I mentioned in the issue?

@danielfeismann
Copy link
Member

Have you look into the failed powerflow messages, like I mentioned in the issue?

We talked about this and I maybe you can shed a bit more light on what changed there (at least to me).

@sebastian-peter
Copy link
Member

@danielfeismann @marvinheintze It used to be the case that failed power flow events were counted by the scheduler. Nowadays the RuntimeEventListener handles that (which fits a bit more nicely). Easier to understand when one checks out where edu.ie3.simona.event.RuntimeEvent.PowerFlowFailed is used (Ctrl-Click in Intellij). But maybe one can go through all other messages sent in SIMONA and check whether the actual protocol still corresponds to the UML documentation.

sebastian-peter
sebastian-peter requested changes Aug 29, 2024
View reviewed changes
Copy link
Member

@sebastian-peter sebastian-peter left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, just realized that the message is still called CompletionMessage for external simulations. Just corrected it myself, and the rest should be dealt with by #898 (thus you can just leave ExtEvSimulationClasses.puml the way it is).

For my comments below, please just reach out right away if I can assist you with the adaptations. It's probably a bit overwhelming when you just started out working on this.

docs/uml/protocol/InitializationPhase.puml Outdated Show resolved Hide resolved
docs/uml/protocol/InitializationPhase.puml Outdated Show resolved Hide resolved
sebastian-peter
sebastian-peter requested changes Sep 3, 2024
View reviewed changes
Copy link
Member

@sebastian-peter sebastian-peter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adapting the initialization diagram, looks great!

Could you adapt all other files in the same fashion as well? Please let me know if you have questions.

docs/uml/protocol/ExtEvSimulationSequence.puml Outdated Show resolved Hide resolved
docs/uml/protocol/ParticipantTriggeredByItself.puml Outdated Show resolved Hide resolved
docs/uml/protocol/ParticipantTriggeredByPrimaryData.puml Outdated Show resolved Hide resolved
docs/uml/protocol/ParticipantTriggeredBySecondaryData.puml Outdated Show resolved Hide resolved
sebastian-peter
sebastian-peter requested changes Sep 17, 2024
View reviewed changes
Copy link
Member

@sebastian-peter sebastian-peter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now there's only a changelog entry missing and we're good to go!

marvinheintze
marvinheintze commented Sep 17, 2024
View reviewed changes
Copy link
Contributor Author

@marvinheintze marvinheintze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit message should be "Changelog entry"

@marvinheintze marvinheintze force-pushed the mh/#675-Adapting-UML-diagrams-and-docs branch from 825be30 to 6bc5def Compare September 17, 2024 08:45
@marvinheintze marvinheintze force-pushed the mh/#675-Adapting-UML-diagrams-and-docs branch from 2791b93 to baf97f8 Compare September 17, 2024 09:02
sebastian-peter
sebastian-peter approved these changes Sep 17, 2024
View reviewed changes
Copy link
Member

@sebastian-peter sebastian-peter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good now, thanks!

@sebastian-peter sebastian-peter merged commit 0adea42 into dev Sep 17, 2024
3 checks passed
@sebastian-peter sebastian-peter deleted the mh/#675-Adapting-UML-diagrams-and-docs branch September 17, 2024 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebastian-peter sebastian-peter sebastian-peter approved these changes

Assignees

@marvinheintze marvinheintze

Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Adapt UML diagrams and docs to changed scheduling protocol
3 participants
@marvinheintze @danielfeismann @sebastian-peter